refactor: reorient CLAIM note consumption flow#2528
Merged
partylikeits1983 merged 55 commits intoagglayerfrom Mar 17, 2026
Merged
refactor: reorient CLAIM note consumption flow#2528partylikeits1983 merged 55 commits intoagglayerfrom
CLAIM note consumption flow#2528partylikeits1983 merged 55 commits intoagglayerfrom
Conversation
partylikeits1983
added
agglayer
partylikeits1983
force-pushed
the
ajl-reorient-claim-note-flow
branch
from
55ee9fb to
57cfbb5
Compare
partylikeits1983
added
the
no changelog
Fumuran
requested changes
Mar 2, 2026
Contributor
Fumuran
left a comment
Fumuran
left a comment
There was a problem hiding this comment.
Looks great, thank you! I left some nits and questions inline.
This is partial review, I only looked through the MASM code (but mostly in a way of code optimization, not the algorithm correctness).
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
Co-authored-by: Andrey Khmuro <[email protected]>
partylikeits1983
force-pushed
the
ajl-reorient-claim-note-flow
branch
from
ee64a9b to
6a602df
Compare
mmagician
reviewed
Mar 14, 2026
mmagician
force-pushed
the
ajl-reorient-claim-note-flow
branch
from
0d19a5e to
cfb6211
Compare
mmagician
requested changes
Mar 14, 2026
* chore: pass faucet ID on stack to verify_claim_amount * chore: pass faucet ID on stack to build_mint_output_note * chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR * chore: drop "y" in verify_u128_to_native_amount_conversion * chore: avoid uplicating y only to drop it later * chore: adjust comments; no longer reading faucetID from mem * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix: rename to output & fix copilot suggestion --------- Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: riemann <[email protected]>
mmagician
reviewed
Mar 16, 2026
Collaborator
mmagician
left a comment
mmagician
left a comment
There was a problem hiding this comment.
The merge changes look good now 👍🏼
I left a few more questions / open items to investigate, but other than those we should be ready to merge soon.
Comment on lines
+41
to
+42
| const CLAIM_PROOF_DATA_WORD_LEN = 134 | ||
| const CLAIM_LEAF_DATA_WORD_LEN = 8 |
Collaborator
There was a problem hiding this comment.
yes it might need some more thinking, but at least we should not duplicate the constants that refer to the same underlying data, which these do
Comment on lines
+644
to
+648
| # Compute note tag targeting the faucet account | ||
| loc_load.BUILD_MINT_FAUCET_PREFIX_LOC | ||
| # => [faucet_prefix, note_type, MINT_RECIPIENT, pad(16)] | ||
|
|
||
| exec.note_tag::create_account_target |
Collaborator
There was a problem hiding this comment.
I don't understand why this is the case, could you pinpoint why this is needed?
IIUC, this is a tag for the faucet, nothing to do with the P2ID that is created later?
The faucet is a network account, and the consumption of notes should not be determined by tag, but by note attachment.
Contributor
Author
Resolved.
I set the tag of the |
mmagician
approved these changes
Mar 17, 2026
partylikeits1983
added a commit
that referenced
this pull request
Mar 17, 2026
* chore: pass faucet ID on stack to verify_claim_amount * chore: pass faucet ID on stack to build_mint_output_note * chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR * chore: drop "y" in verify_u128_to_native_amount_conversion * chore: avoid uplicating y only to drop it later * chore: adjust comments; no longer reading faucetID from mem * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix: rename to output & fix copilot suggestion --------- Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: riemann <[email protected]>
Fumuran
added a commit
that referenced
this pull request
Mar 17, 2026
* feat: wip CLAIM note flow reorientation * wip: build MINT note from aggbridge * wip: created MINT note encodes incorrect public P2ID or public P2ID not in DataStore * feat: working e2e CLAIM flow reordering * fix: cleanup rm debug.stack * refactor: improve readability of MINT note memory addresses * refactor: fix stack comments * fix: rm println statements * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/note_scripts/CLAIM.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * Update crates/miden-agglayer/asm/agglayer/bridge/bridge_in.masm Co-authored-by: Andrey Khmuro <[email protected]> * feat: add CGI chain hash computation * test: [WiP-1] first iteration, not building * test: [WiP-2] building state * test: updated script, failing * test: update script (passing) * refactor: improve bridge_in doc comments * refactor: add constants for memory addresses * refactor: cleanup memory layout in bridge_in * fix: update comment in CLAIM note * refactor: remove redundant dropw * refactor: use execution_hint::ALWAYS * refactor: rm redundant const note type * refactor: simplify loc_store/load ops in bridge_in * refactor: decompose build_mint_output_note into modular helpers * refactor: remove pad(x) stack comments from exec'ed procs * feat: add constants for attachement types * refactor: create ctorage slots for CGI chain hash, use actiual procedure in test * refactor: incapsulate CGI hash computation * fix: add call to verify_u256_scale_down procedure * refactor: use ATTACHMENT_KIND_NONE const * fix: use truncate_stack proc for FPI call & exec compatibility * refactor: add get_scale & get_scale_inner for call vs exec * test: update solidity contracts to generate CGI chain hash. Test is failing * test: fix bug, test is passing * chore: fix doc format * refactor: use local memory for TOKEN_ADDR_HASH_PTR * refactor: look up faucet by token address only once during claim * fix: fix array comment syntax * refactor: rm pad comments from exec'ed proc * refactor: rm swapw & replace w padw * fix: update comment * refactor: refactor is_token_registered proc * chore: more explicit mem constant names * fix: explicit pad before FPI call * refactor: remove unused bridge account ID storage slot from AggLayer faucet * chore: opinionated improvements to #2528 (#2602) * chore: pass faucet ID on stack to verify_claim_amount * chore: pass faucet ID on stack to build_mint_output_note * chore: completely remove CLAIM_FAUCET_ID_PREFIX_MEM_ADDR * chore: drop "y" in verify_u128_to_native_amount_conversion * chore: avoid uplicating y only to drop it later * chore: adjust comments; no longer reading faucetID from mem * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix: rename to output & fix copilot suggestion --------- Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: riemann <[email protected]> * refactor: move bridge & faucet code from lib.rs to dedicated modules * refactor: remove unused code, update list of bridge storage slots, update docs * refactor: create constants for local memory offsets * refactor: move verifyLeafBridgeHarness to helpers, remove unused code, remove local memory usage for leaf value * test: generate CGI chain hash during ClaimAssetTestVectorsRollupTx --------- Co-authored-by: riemann <[email protected]> Co-authored-by: Alexander John Lee <[email protected]> Co-authored-by: Marti <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]>
mmagician
pushed a commit
that referenced
this pull request
Mar 17, 2026
Merge 3 agglayer commits: claim flow reorientation (#2528), rollup claim processing (#2573), and global index chain hash (#2516). Conflict resolution: - bridge_config.masm: take agglayer (token registry additions) - faucet/mod.masm: take agglayer (claim removal, get_scale refactor), keep our get_metadata_hash procedure and storage slots - components/faucet.masm: export both get_metadata_hash and get_scale - faucet.rs: take agglayer ownable pattern (owner_config_slot), keep metadata hash slots and accessors Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR reorients the flow in which the
CLAIMnote is consumed.This PR reorients the flow from:
CLAIM note is consumed by AggFaucet account, which does an FPI call to bridge to verify merkle proof, if valid creates P2ID note
to:
CLAIM note is consumed by AggBridge account which verifies the merkle proof, if valid creates a MINT note, which is then consumed by AggFaucet account in separate tx, which creates P2ID note.
This PR adds a mapping to the AggBridge which stores the
bytes20origin token address in the faucet registry ashash(bytes20)->AccountId.This is so that when creating the
MINTnote, the correct network note attachment target can be set. Storing the hash of the destination address as the key in a mapping, where the value is the aggfaucetAccountIdis essential, as previously we only stored registered AggFaucets in a mapping like this:AccountId->bool.Note: Both the
MINTnote &P2IDnote which created by the consumption of theMINTnote, both use thePROOF_DATA_KEY(hash of proof data) as their serial numbers.Resolves #2506